pastk/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[android] Refactor handling of links on PlacePage

Browse source 
- Make PlacePageLinksFragment.getLink() non-static as
  requested on one of the previous code reviews.
- Move getWebsiteUrl() and getKayakUrl() to MapObject.
- Force @NonNull String invariant for getKayakUrl().
  Currently all getMetadata() methods return @NonNull Strings.
  Replacing empty strings with null objects and removing TextUtils.isEmpty()
  clutter is very error-prone and requires a separate refactoring.
- Fix few lint warnings.

Signed-off-by: Roman Tsisyk <roman@tsisyk.com>
This commit is contained in:
Roman Tsisyk 2023-12-03 18:00:26 +02:00
parent de2b623fed
commit bef4ba8d22
7 changed files with 101 additions and 141 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
android/app/src/main/java/app/organicmaps/bookmarks/data/MapObject.java
View file

@ -9,15 +9,19 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.core.os.ParcelCompat;
import app.organicmaps.Framework;
import app.organicmaps.routing.RoutePointInfo;
import app.organicmaps.search.Popularity;
import app.organicmaps.util.Utils;
import app.organicmaps.widget.placepage.PlacePageData;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
import java.util.Objects;
// TODO(yunikkk): Refactor. Displayed information is different from edited information, and it's better to
// separate them. Simple getters from jni place_page::Info and osm::EditableFeature should be enough.
@ -46,6 +50,9 @@ public class MapObject implements PlacePageData
public static final int OPENING_MODE_DETAILS = 2;
public static final int OPENING_MODE_FULL = 3;
private static String kHttp = "http://";
private static String kHttps = "https://";
@NonNull
private final FeatureId mFeatureId;
@MapObjectType
@ -54,10 +61,11 @@ public class MapObject implements PlacePageData
private String mTitle;
@Nullable
private final String mSecondaryTitle;
private String mSubtitle;
private final String mSubtitle;
private double mLat;
private double mLon;
private final String mAddress;
@NonNull
private final Metadata mMetadata;
private final String mApiId;
private final RoutePointInfo mRoutePointInfo;
@ -98,7 +106,7 @@ public class MapObject implements PlacePageData
mAddress = address;
mLat = lat;
mLon = lon;
mMetadata = metadata;
mMetadata = metadata != null ? metadata : new Metadata();
mApiId = apiId;
mRoutePointInfo = routePointInfo;
mOpeningMode = openingMode;
@ -111,7 +119,7 @@ public class MapObject implements PlacePageData
protected MapObject(@MapObjectType int type, Parcel source)
{
this(ParcelCompat.readParcelable(source, FeatureId.class.getClassLoader(), FeatureId.class), // FeatureId
this(Objects.requireNonNull(ParcelCompat.readParcelable(source, FeatureId.class.getClassLoader(), FeatureId.class)), // FeatureId
type, // MapObjectType
source.readString(), // Title
source.readString(), // SecondaryTitle
@ -123,8 +131,8 @@ public class MapObject implements PlacePageData
source.readString(), // ApiId;
ParcelCompat.readParcelable(source, RoutePointInfo.class.getClassLoader(), RoutePointInfo.class), // RoutePointInfo
source.readInt(), // mOpeningMode
ParcelCompat.readParcelable(source, Popularity.class.getClassLoader(), Popularity.class),
source.readString(),
Objects.requireNonNull(ParcelCompat.readParcelable(source, Popularity.class.getClassLoader(), Popularity.class)),
Objects.requireNonNull(source.readString()),
source.readInt(),
null // mRawTypes
);
@ -258,9 +266,30 @@ public class MapObject implements PlacePageData
return res == null ? "" : res;
}
public boolean hasMetadata()
@NonNull
public String getWebsiteUrl(boolean strip)
{
return mMetadata != null && !mMetadata.isEmpty();
final String website = getMetadata(Metadata.MetadataType.FMD_WEBSITE);
final int len = website.length();
if (strip && len > 1)
{
final int start = website.startsWith(kHttps) ? kHttps.length() : (website.startsWith(kHttp) ? kHttp.length() : 0);
final int end = website.endsWith("/") ? len - 1 : len;
return website.substring(start, end);
}
return website;
}
@NonNull
public String getKayakUrl()
{
final String uri = getMetadata(Metadata.MetadataType.FMD_EXTERNAL_URI);
if (TextUtils.isEmpty(uri))
return "";
final Date firstDay = new Date();
final Date lastDay = new Date(firstDay.getTime() + (1000 * 60 * 60 * 24));
final String res = Framework.nativeGetKayakHotelLink(Utils.getCountryCode(), uri, firstDay, lastDay);
return res == null ? "" : res;
}
public String getApiId()
@ -306,11 +335,6 @@ public class MapObject implements PlacePageData
return mMapObjectType == BOOKMARK;
}
public final boolean isApiPoint()
{
return mMapObjectType == API_POINT;
}
@Nullable
public RoutePointInfo getRoutePointInfo()
{
@ -385,7 +409,7 @@ public class MapObject implements PlacePageData
return mFeatureId.hashCode();
}
public static final Creator<MapObject> CREATOR = new Creator<MapObject>()
public static final Creator<MapObject> CREATOR = new Creator<>()
{
@Override
public MapObject createFromParcel(Parcel source)

5
android/app/src/main/java/app/organicmaps/bookmarks/data/Metadata.java
View file

@ -97,11 +97,6 @@ public class Metadata implements Parcelable
return mMetadataMap.get(type);
}
boolean isEmpty()
{
return mMetadataMap.isEmpty();
}
@Override
public int describeContents()
{

16
android/app/src/main/java/app/organicmaps/car/screens/PlaceScreen.java
View file

@ -6,6 +6,7 @@ import static android.text.Spanned.SPAN_INCLUSIVE_INCLUSIVE;
import android.content.Intent;
import android.net.Uri;
import android.text.SpannableString;
import android.text.TextUtils;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
@ -198,9 +199,10 @@ public class PlaceScreen extends BaseMapScreen implements OnBackPressedCallback.
{
Objects.requireNonNull(mMapObject);
final String phoneNumber = getFirstPhoneNumber(mMapObject);
if (!phoneNumber.isEmpty())
final String phones = mMapObject.getMetadata(Metadata.MetadataType.FMD_PHONE_NUMBER);
if (!TextUtils.isEmpty(phones))
{
final String phoneNumber = phones.split(";", 1)[0];
final Action.Builder openDialBuilder = new Action.Builder();
openDialBuilder.setIcon(new CarIcon.Builder(IconCompat.createWithResource(getCarContext(), R.drawable.ic_phone)).build());
openDialBuilder.setOnClickListener(() -> getCarContext().startCarApp(new Intent(Intent.ACTION_DIAL, Uri.parse("tel:" + phoneNumber))));
@ -242,16 +244,6 @@ public class PlaceScreen extends BaseMapScreen implements OnBackPressedCallback.
mRoutingController.rebuildLastRoute();
}
@NonNull
private static String getFirstPhoneNumber(@NonNull MapObject mapObject)
{
if (!mapObject.hasMetadata())
return "";
final String phones = mapObject.getMetadata(Metadata.MetadataType.FMD_PHONE_NUMBER);
return phones.split(";", 1)[0];
}
@Override
public void onBackPressed()
{

3
android/app/src/main/java/app/organicmaps/car/util/UiHelpers.java
View file

@ -103,9 +103,6 @@ public final class UiHelpers
@Nullable
public static Row getPlaceOpeningHoursRow(@NonNull MapObject place, @NonNull CarContext context)
{
if (!place.hasMetadata())
return null;
final String ohStr = place.getMetadata(Metadata.MetadataType.FMD_OPEN_HOURS);
final Timetable[] timetables = OpeningHours.nativeTimetablesFromString(ohStr);
final boolean isEmptyTT = (timetables == null || timetables.length == 0);

11
android/app/src/main/java/app/organicmaps/widget/placepage/PlacePageView.java
View file

@ -144,13 +144,12 @@ public class PlacePageView extends Fragment implements View.OnClickListener,
private PlacePageViewModel mViewModel;
private MapObject mMapObject;
private static void refreshMetadataOrHide(String metadata, View metaLayout, TextView metaTv)
private static void refreshMetadataOrHide(@Nullable String metadata, @NonNull View metaLayout, @NonNull TextView metaTv)
{
if (!TextUtils.isEmpty(metadata))
{
metaLayout.setVisibility(VISIBLE);
if (metaTv != null)
metaTv.setText(metadata);
metaTv.setText(metadata);
}
else
metaLayout.setVisibility(GONE);
@ -319,14 +318,14 @@ public class PlacePageView extends Fragment implements View.OnClickListener,
private void updateLinksView()
{
final boolean hasLinkAvailable = PlacePageLinksFragment.hasLinkAvailable(mMapObject);
updateViewFragment(PlacePageLinksFragment.class, LINKS_FRAGMENT_TAG, R.id.place_page_links_fragment, hasLinkAvailable);
updateViewFragment(PlacePageLinksFragment.class, LINKS_FRAGMENT_TAG, R.id.place_page_links_fragment, true);
}
private void updateOpeningHoursView()
{
final String ohStr = mMapObject.getMetadata(Metadata.MetadataType.FMD_OPEN_HOURS);
updateViewFragment(PlacePageOpeningHoursFragment.class, OPENING_HOURS_FRAGMENT_TAG, R.id.place_page_opening_hours_fragment, !ohStr.isEmpty());
updateViewFragment(PlacePageOpeningHoursFragment.class, OPENING_HOURS_FRAGMENT_TAG,
R.id.place_page_opening_hours_fragment, !TextUtils.isEmpty(ohStr));
}
private void updatePhoneView()

149
android/app/src/main/java/app/organicmaps/widget/placepage/sections/PlacePageLinksFragment.java
View file

@ -20,15 +20,11 @@ import app.organicmaps.bookmarks.data.MapObject;
import app.organicmaps.bookmarks.data.Metadata;
import app.organicmaps.util.UiUtils;
import app.organicmaps.util.Utils;
import app.organicmaps.util.log.Logger;
import app.organicmaps.widget.placepage.PlacePageUtils;
import app.organicmaps.widget.placepage.PlacePageViewModel;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
import java.util.Objects;
import static android.view.View.GONE;
import static android.view.View.VISIBLE;
@ -37,17 +33,6 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
{
private static final String TAG = PlacePageLinksFragment.class.getSimpleName();
static final List<Metadata.MetadataType> supportedLinks = Arrays.asList(
Metadata.MetadataType.FMD_WEBSITE,
Metadata.MetadataType.FMD_EMAIL,
Metadata.MetadataType.FMD_WIKIMEDIA_COMMONS,
Metadata.MetadataType.FMD_EXTERNAL_URI,
Metadata.MetadataType.FMD_CONTACT_FACEBOOK,
Metadata.MetadataType.FMD_CONTACT_INSTAGRAM,
Metadata.MetadataType.FMD_CONTACT_TWITTER,
Metadata.MetadataType.FMD_CONTACT_VK,
Metadata.MetadataType.FMD_CONTACT_LINE);
private View mFrame;
private View mFacebookPage;
private TextView mTvFacebookPage;
@ -71,51 +56,40 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
private PlacePageViewModel mViewModel;
private MapObject mMapObject;
private static void refreshMetadataOrHide(String metadata, View metaLayout, TextView metaTv)
private static void refreshMetadataOrHide(@Nullable String metadata, @NonNull View metaLayout,
@NonNull TextView metaTv)
{
if (!TextUtils.isEmpty(metadata))
{
metaLayout.setVisibility(VISIBLE);
if (metaTv != null)
metaTv.setText(metadata);
metaTv.setText(metadata);
}
else
metaLayout.setVisibility(GONE);
}
private static String getLink(@NonNull MapObject mapObject, Metadata.MetadataType type)
@NonNull
private String getLink(@NonNull Metadata.MetadataType type)
{
final String metadata = mapObject.getMetadata(type);
if (TextUtils.isEmpty(metadata))
return "";
switch (type)
{
case FMD_EXTERNAL_URI:
return getExternalUrl(mapObject);
return mMapObject.getKayakUrl();
case FMD_WEBSITE:
return getWebsiteUrl(mapObject, false /* strip */);
return mMapObject.getWebsiteUrl(false /* strip */);
case FMD_CONTACT_FACEBOOK:
case FMD_CONTACT_INSTAGRAM:
case FMD_CONTACT_TWITTER:
case FMD_CONTACT_VK:
case FMD_CONTACT_LINE:
if (TextUtils.isEmpty(mMapObject.getMetadata(type)))
return "";
return Framework.nativeGetPoiContactUrl(type.toInt());
default:
return metadata;
return mMapObject.getMetadata(type);
}
}
public static boolean hasLinkAvailable(@NonNull MapObject mapObject)
{
for (Metadata.MetadataType type: supportedLinks) {
final String metadata = getLink(mapObject, type);
if (!TextUtils.isEmpty(metadata))
return true;
}
return false;
}
@Nullable
@Override
public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState)
@ -132,13 +106,15 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
mKayak = mFrame.findViewById(R.id.ll__place_kayak);
mKayak.setOnClickListener((v) -> {
final String url = Objects.requireNonNull(getExternalUrl(mMapObject));
final String url = mMapObject.getKayakUrl();
final MwmActivity activity = (MwmActivity) requireActivity();
activity.openKayakLink(url);
if (!TextUtils.isEmpty(url))
activity.openKayakLink(url);
});
mKayak.setOnLongClickListener((v) -> {
final String url = Objects.requireNonNull(getExternalUrl(mMapObject));
PlacePageUtils.copyToClipboard(requireContext(), mFrame, url);
final String url = mMapObject.getKayakUrl();
if (!TextUtils.isEmpty(url))
PlacePageUtils.copyToClipboard(requireContext(), mFrame, url);
return true;
});
@ -149,8 +125,11 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
mEmail = mFrame.findViewById(R.id.ll__place_email);
mTvEmail = mFrame.findViewById(R.id.tv__place_email);
mEmail.setOnClickListener((v) -> Utils.sendTo(requireContext(), getLink(mMapObject,
Metadata.MetadataType.FMD_EMAIL)));
mEmail.setOnClickListener(v -> {
final String email = mMapObject.getMetadata(Metadata.MetadataType.FMD_EMAIL);
if (!TextUtils.isEmpty(email))
Utils.sendTo(requireContext(), email);
});
mEmail.setOnLongClickListener((v) -> copyUrl(mEmail, Metadata.MetadataType.FMD_EMAIL));
mWikimedia = mFrame.findViewById(R.id.ll__place_wikimedia);
@ -184,30 +163,26 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
mLinePage.setOnLongClickListener((v) -> copyUrl(mLinePage, Metadata.MetadataType.FMD_CONTACT_LINE));
}
private boolean isSocialUsername(Metadata.MetadataType type)
{
return !mMapObject.getMetadata(type).contains("/");
}
private void openUrl(Metadata.MetadataType type)
{
final String url = getLink(mMapObject, type);
final String url = getLink(type);
if (!TextUtils.isEmpty(url))
Utils.openUrl(requireContext(), url);
}
private boolean copyUrl(View view, Metadata.MetadataType type)
{
final String url = getLink(mMapObject, type);
final String url = getLink(type);
if (TextUtils.isEmpty(url))
return false;
final List<String> items = new ArrayList<>();
items.add(url);
final String metadata = type == Metadata.MetadataType.FMD_WEBSITE
? getWebsiteUrl(mMapObject, false /* strip */)
: mMapObject.getMetadata(type);
final String title = type == Metadata.MetadataType.FMD_WEBSITE ?
mMapObject.getWebsiteUrl(false /* strip */) : mMapObject.getMetadata(type);
// Add user names for social media if available
if (!metadata.equals(url) && isSocialUsername(type))
items.add(metadata);
if (!TextUtils.isEmpty(title) && !title.equals(url) && !title.contains("/"))
items.add(title);
if (items.size() == 1)
PlacePageUtils.copyToClipboard(requireContext(), mFrame, items.get(0));
@ -216,59 +191,33 @@ public class PlacePageLinksFragment extends Fragment implements Observer<MapObje
return true;
}
private void refreshSocialPageLink(@NonNull MapObject mapObject, View view, TextView tvSocialPage, Metadata.MetadataType metaType)
{
final String socialPage = mapObject.getMetadata(metaType);
refreshMetadataOrHide(socialPage, view, tvSocialPage);
}
@Nullable
public static String getExternalUrl(@NonNull MapObject mapObject)
{
final String uri = mapObject.getMetadata(Metadata.MetadataType.FMD_EXTERNAL_URI);
if (TextUtils.isEmpty(uri))
return null;
final Date firstDay = new Date();
final Date lastDay = new Date(firstDay.getTime() + (1000 * 60 * 60 * 24));
final String kayakUri = Framework.nativeGetKayakHotelLink(Utils.getCountryCode(), uri, firstDay, lastDay);
if (kayakUri == null)
{
Logger.w(TAG, "Invalid Kayak URI: " + uri);
return null;
}
return kayakUri;
}
private static String kHttp = "http://";
private static String kHttps = "https://";
private static String getWebsiteUrl(MapObject mapObject, boolean strip)
{
final String website = mapObject.getMetadata(Metadata.MetadataType.FMD_WEBSITE);
final int len = website.length();
if (strip && len > 1)
{
final int start = website.startsWith(kHttps) ? kHttps.length() : (website.startsWith(kHttp) ? kHttp.length() : 0);
final int end = website.endsWith("/") ? len - 1 : len;
return website.substring(start, end);
}
return website;
}
private void refreshLinks()
{
UiUtils.showIf(getExternalUrl(mMapObject) != null, mKayak);
refreshMetadataOrHide(getWebsiteUrl(mMapObject, true /* strip */), mWebsite, mTvWebsite);
UiUtils.showIf(!TextUtils.isEmpty(mMapObject.getKayakUrl()), mKayak);
refreshMetadataOrHide(mMapObject.getWebsiteUrl(true /* strip */), mWebsite, mTvWebsite);
String wikimedia_commons = mMapObject.getMetadata(Metadata.MetadataType.FMD_WIKIMEDIA_COMMONS);
String wikimedia_commons_text = TextUtils.isEmpty(wikimedia_commons) ? "" : getResources().getString(R.string.wikimedia_commons);
refreshMetadataOrHide(wikimedia_commons_text, mWikimedia, mTvWikimedia);
refreshMetadataOrHide(mMapObject.getMetadata(Metadata.MetadataType.FMD_EMAIL), mEmail, mTvEmail);
refreshSocialPageLink(mMapObject, mFacebookPage, mTvFacebookPage, Metadata.MetadataType.FMD_CONTACT_FACEBOOK);
refreshSocialPageLink(mMapObject, mInstagramPage, mTvInstagramPage, Metadata.MetadataType.FMD_CONTACT_INSTAGRAM);
refreshSocialPageLink(mMapObject, mTwitterPage, mTvTwitterPage, Metadata.MetadataType.FMD_CONTACT_TWITTER);
refreshSocialPageLink(mMapObject, mVkPage, mTvVkPage, Metadata.MetadataType.FMD_CONTACT_VK);
refreshSocialPageLink(mMapObject, mLinePage, mTvLinePage, Metadata.MetadataType.FMD_CONTACT_LINE);
final String facebook = mMapObject.getMetadata(Metadata.MetadataType.FMD_CONTACT_FACEBOOK);
refreshMetadataOrHide(facebook, mFacebookPage, mTvFacebookPage);
final String instagram = mMapObject.getMetadata(Metadata.MetadataType.FMD_CONTACT_INSTAGRAM);
refreshMetadataOrHide(instagram, mInstagramPage, mTvInstagramPage);
final String twitter = mMapObject.getMetadata(Metadata.MetadataType.FMD_CONTACT_TWITTER);
refreshMetadataOrHide(twitter, mTwitterPage, mTvTwitterPage);
final String vk = mMapObject.getMetadata(Metadata.MetadataType.FMD_CONTACT_VK);
refreshMetadataOrHide(vk, mVkPage, mTvVkPage);
final String line = mMapObject.getMetadata(Metadata.MetadataType.FMD_CONTACT_LINE);
refreshMetadataOrHide(line, mLinePage, mTvLinePage);
final String kayak = mMapObject.getKayakUrl();
UiUtils.showIf(!TextUtils.isEmpty(kayak), mKayak);
}
@Override

8
android/app/src/main/java/app/organicmaps/widget/placepage/sections/PlacePageWikipediaFragment.java
View file

@ -105,9 +105,13 @@ public class PlacePageWikipediaFragment extends Fragment implements Observer<Map
}
final String wikipediaLink = mMapObject.getMetadata(Metadata.MetadataType.FMD_WIKIPEDIA);
mWiki.setOnClickListener((v) -> Utils.openUrl(requireContext(), wikipediaLink));
mWiki.setOnClickListener((v) -> {
if (!TextUtils.isEmpty(wikipediaLink))
Utils.openUrl(requireContext(), wikipediaLink);
});
mWiki.setOnLongClickListener((v) -> {
PlacePageUtils.copyToClipboard(requireContext(), mFrame, wikipediaLink);
if (!TextUtils.isEmpty(wikipediaLink))
PlacePageUtils.copyToClipboard(requireContext(), mFrame, wikipediaLink);
return true;
});
}